-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support to_timestamp
#838
Support to_timestamp
#838
Conversation
Codecov Report
@@ Coverage Diff @@
## main #838 +/- ##
=======================================
Coverage 75.50% 75.51%
=======================================
Files 73 73
Lines 3985 4011 +26
Branches 713 721 +8
=======================================
+ Hits 3009 3029 +20
+ Misses 818 817 -1
- Partials 158 165 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Co-authored-by: Charles Blackmon-Luca <20627856+charlesbluca@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With #857 closed in favor of this, would you mind adding this parser.rs
test in?
I'm not quite sure why |
I saw this pop up on a some tests unrelated to this pr: https://github.com/dask-contrib/dask-sql/actions/runs/3292477865/attempts/1. They seem to be presenting only on Mac-os so far and seems intermittent. Might need to investigate further if this keeps showing up. |
Co-authored-by: Charles Blackmon-Luca <20627856+charlesbluca@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments below.
Also looking through the postgresql docs the integer version of this method doesn't accept a format
argument. Do you think it's possible to update the parser to handle this? If not we might need to either throw an error or ignore the format for integer cases.
Another note is that the format
argument for postgresql expects a different formatting pattern than python's datetime. For now we should be good to use the python format but add that to the docs
Interesting, thanks! Yes, this should be easy to fix, so I can update this as well. |
Co-authored-by: Ayush Dattagupta <ayushdg95@gmail.com>
Closes #831.